Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update trigger for DTD deployment job. #3585

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Update trigger for DTD deployment job. #3585

merged 3 commits into from
Nov 26, 2024

Conversation

kt86
Copy link
Contributor

@kt86 kt86 commented Nov 26, 2024

The previous script was never run (which seems to be the cause for #3583), so there seems to be an issue with the triggers:

Therefore I changed the following:

  • Run on merged PRs.
  • fix syntax …for target path.

@mrieser : Can you have a look in it, and if ok, merge it?

And then I assume, we need a PR, changing something in the dtd path, to see, if it works (and updating the ev's dtd at the same time) ...

Update: I have prepared such a PR: #3586

@mrieser
Copy link
Contributor

mrieser commented Nov 26, 2024

I don't have much experience with GitHub actions. Comparing your suggested changes with https://github.com/matsim-org/matsim-libs/blob/master/.github/workflows/deploy-on-pr-merge.yaml I see that the deploy_on_pr_merge also has the condition that it is only triggered when merging into master branch. Currently, the dtd-deploy would be triggered on ANY merge request. Do we want that?

Otherwise... as I said, I don't have much experience, so it's okay for me. I guess, we'l have to try it out and see what happens.

@kt86
Copy link
Contributor Author

kt86 commented Nov 26, 2024

As far as I understand it, I changed it in a way, that the workflow is triggered on every closed PR, that modify something in the path 'matsim/src/main/resources/dtd/**'.

This is currently true only few PRs.

And then the job is only executed if the close was triggered by a merging.

Do we have any PRs not merging into master?

Unfortunately, I am also not the expert of it, but from some reading today, I am not totally sure, with that branches setting here. But yes, we can try to put it in again.

@jfbischoff jfbischoff merged commit 22e891b into master Nov 26, 2024
49 checks passed
@jfbischoff jfbischoff deleted the FixDtdPublication branch November 26, 2024 14:45
@jfbischoff
Copy link
Collaborator

The pipeline is now running, unfortunately something seems wrong with the paths for rsync:

rsync: [sender] change_dir "/github/workspace/matsim/src/main/resources/dtd" failed: No such file or directory (2)

Here is the full log:
https://github.com/matsim-org/matsim-libs/actions/runs/12039650110/job/33567897764

@jfbischoff
Copy link
Collaborator

jfbischoff commented Nov 27, 2024

I think the path should probably be "/github/workspace/matsim-libs/matsim/src/main/resources/dtd

@kt86
Copy link
Contributor Author

kt86 commented Nov 27, 2024

I think the path should probably be "/github/workspace/matsim-libs/matsim/src/main/resources/dtd

Can you just change it, please?
Unfortunately I have no idea, with the path information. I am just happy that it has worked to start the pipeline;)

@kt86
Copy link
Contributor Author

kt86 commented Nov 27, 2024

Now it works :)

grafik

The final solution to ass the checkout command as well: #3597

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants